Feature - ChartView Pan Ended Delegate Call #3612

AntonTheDev · 2018-08-27T12:44:59Z

Issue Link 🔗

Over the years, every I have come to implement this library, i have found that 1 feature for me has always been missing which is the ability to know when the user stopped panning across a chart to display values on the fly, and have it reset once the user lets go. I've yet to find a clean solution other than modifying the library, and I have added some callback logic to the ChartViewDelegate

Goals ⚽

To be able to install this from the master repo, and not always have to fork it to add this exact feature./

Implementation Details 🚧

Added and optional method to the ChartViewDelegate to tell the delegate that the pan gesture has reached a cancelled / ended state

public protocol ChartViewDelegate
{
     ...

    /// Called when a user stop highlighting values while panning
    @objc optional func chartViewDidEndPanning(_ chartView: ChartViewBase)
}

Testing Details 🔍

None, there are no specific tests. This should not effect the snapshotting at all, and has no effect on the existing codebase other than an optional callback.

…ially allow users to manually reset the hightlight values once panning is complete

codecov-io · 2018-08-27T12:54:47Z

Codecov Report

Merging #3612 into master will increase coverage by 1.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3612      +/-   ##
==========================================
+ Coverage   29.94%   31.01%   +1.06%     
==========================================
  Files         118      114       -4     
  Lines       13793    10483    -3310     
==========================================
- Hits         4130     3251     -879     
+ Misses       9663     7232    -2431

Impacted Files	Coverage Δ
Source/Charts/Charts/BarLineChartViewBase.swift	`65.78% <ø> (+39.25%)`	⬆️
Source/Charts/Charts/ChartViewBase.swift	`19.51% <ø> (-2.08%)`	⬇️
Source/Charts/Utils/Platform.swift	`0% <0%> (-15.39%)`	⬇️
Source/Charts/Charts/PieRadarChartViewBase.swift	`5.63% <0%> (-14.07%)`	⬇️
Source/Charts/Charts/CombinedChartView.swift	`25% <0%> (-13.62%)`	⬇️
...s/Data/Implementations/Standard/PieChartData.swift	`18.91% <0%> (-13.29%)`	⬇️
...s/Data/Implementations/Standard/ChartDataSet.swift	`23.33% <0%> (-12.94%)`	⬇️
Source/Charts/Utils/ChartUtils.swift	`36.87% <0%> (-9.24%)`	⬇️
Source/Charts/Utils/ViewPortHandler.swift	`18.13% <0%> (-8.72%)`	⬇️
Source/Charts/Utils/Platform+Accessibility.swift	`31.57% <0%> (-5.46%)`	⬇️
... and 63 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fca1f7...33e8eb1. Read the comment docs.

jjatie · 2018-09-16T17:05:00Z

@liuxuan30 @petester42 This looks good to me!

jjatie · 2018-09-16T17:07:49Z

Source/Charts/Charts/ChartViewBase.swift

@@ -25,6 +25,9 @@ public protocol ChartViewDelegate
    /// - parameter highlight: The corresponding highlight object that contains information about the highlighted position such as dataSetIndex etc.
    @objc optional func chartValueSelected(_ chartView: ChartViewBase, entry: ChartDataEntry, highlight: Highlight)

+    /// Called when a user stop highlighting values while panning


I believe your documentation needs to be updated. Shouldn't this read something like Called when a user stops panning?

liuxuan30

documentations need to be precise.

If many people want the delegate, we could merge.

proped · 2018-10-10T05:58:59Z

I need this. Could u merged it ?

Updated documentation in the code based on feedback

AntonTheDev · 2018-10-10T13:01:07Z

I updated the comment accordingly in the code, hope this helps :)

natan · 2018-10-23T23:15:24Z

@liuxuan30 Any update on this?

pmairoldi · 2018-10-23T23:24:54Z

These changes seem fine to me. If we’re going add delegate calls for gesture recognizes we should think about if we need them for all states of the gestures. Do people care about when a pan starts, or when it is ongoing?

liuxuan30 · 2018-10-24T01:46:02Z

I'm open to merge.but as @petester42 said, if we add this, maybe we should add more?

natan · 2018-10-24T16:47:02Z

@liuxuan30 @AntonTheDev Great, thank you for moving this along!

liuxuan30 · 2018-10-25T01:09:41Z

shall we add at least didStartPanning to pair for this PR?

AntonTheDev · 2018-10-26T21:17:26Z

I believe the whole "didStartPanning" can be be assumed when "chartValueSelected" is called? Since you can't really start panning without making a callback. But once you take your finger off the screen, there was no way to tell that you did. The values do no unselect themselves

anton-filimonov · 2018-10-27T05:12:01Z

I believe it'd be useful to make the callbacks similar to what we have for scrollview. And actually call them in pretty much the same situations as ScrollView does.

jjatie · 2018-10-27T10:34:12Z

I second making the delegate behave and be named as closely to UIScrollViewDelegate as possible.

liuxuan30 · 2018-11-01T01:46:45Z

@AntonTheDev I'm ok to merge this, but it's just strange to me that we had didEnd without didStart. Though you might be able to know it get started through chartValueSelected, I think it's still different events.

How about we append the didStartPanning in recognizer.state == NSUIGestureRecognizerState.began closure?

Arkezis · 2019-01-17T08:50:20Z

Thanks for your awesome library ! It is really helping us :)
This PR would be awesome for our application, we are looking forward to merge it :)

liuxuan30 · 2019-01-22T00:53:47Z

@petester42 @jjatie how about this PR? shall we add a few more or just merge? I'm looking for a few PRs that can be merged soon and release a new version next week.

jlubeck · 2019-11-19T22:50:49Z

@AntonTheDev I'm ok to merge this, but it's just strange to me that we had didEnd without didStart. Though you might be able to know it get started through chartValueSelected, I think it's still different events.

How about we append the didStartPanning in recognizer.state == NSUIGestureRecognizerState.began closure?

Yes please! I'm running into this situation myself right now. chartValueSelected get called both on drag AND tap, but chartViewDidEndPanning only gets called when the user stops dragging. Definitely different use cases...

Specifically, when this chart gets embedded inside a UIScrollView, we want to stop the UIScrollView from dragging when we are dragging on the chart. So one would think we can stop the UIScrollView from dragging on chartValueSelected, and restart it on chartViewDidEndPanning

But the problem with this solution is that chartValueSelected is also called on a single tap on the chart. So when this happens, the UIScrollView stops dragging, but then we have no callback to restart it... And the user is stuck with an unscrollable UIScrollView until get actually drags on the Chart.

Hope this was clear. I'm about to leave for the day, but if I don't hear from anyone back I'll probably make a pull request myself tomorrow.

AntonTheDev added 2 commits August 26, 2018 17:48

Added delegate callback to detect when panning is finished, to potent…

767d03d

…ially allow users to manually reset the hightlight values once panning is complete

Changed the part of the code where the delegate gets called

c3d3ed7

dtroupe18 mentioned this pull request Sep 10, 2018

touchesEnded event in BarChartView not called #613

Closed

jjatie reviewed Sep 16, 2018

View reviewed changes

liuxuan30 requested changes Sep 19, 2018

View reviewed changes

Update ChartViewBase.swift

33e8eb1

Updated documentation in the code based on feedback

liuxuan30 approved these changes Oct 24, 2018

View reviewed changes

jjatie approved these changes Jan 22, 2019

View reviewed changes

liuxuan30 merged commit f2795b9 into ChartsOrg:master Jan 22, 2019

liuxuan30 mentioned this pull request Jan 22, 2019

Know when user is done interacting with graph #3599

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature - ChartView Pan Ended Delegate Call #3612

Feature - ChartView Pan Ended Delegate Call #3612

AntonTheDev commented Aug 27, 2018 •

edited

Loading

codecov-io commented Aug 27, 2018 •

edited

Loading

jjatie commented Sep 16, 2018

jjatie Sep 16, 2018

liuxuan30 left a comment •

edited

Loading

proped commented Oct 10, 2018

AntonTheDev commented Oct 10, 2018

natan commented Oct 23, 2018

pmairoldi commented Oct 23, 2018 •

edited

Loading

liuxuan30 commented Oct 24, 2018

natan commented Oct 24, 2018

liuxuan30 commented Oct 25, 2018

AntonTheDev commented Oct 26, 2018 •

edited

Loading

anton-filimonov commented Oct 27, 2018

jjatie commented Oct 27, 2018

liuxuan30 commented Nov 1, 2018 •

edited

Loading

Arkezis commented Jan 17, 2019

liuxuan30 commented Jan 22, 2019

jlubeck commented Nov 19, 2019

Feature - ChartView Pan Ended Delegate Call #3612

Feature - ChartView Pan Ended Delegate Call #3612

Conversation

AntonTheDev commented Aug 27, 2018 • edited Loading

Issue Link 🔗

Goals ⚽

Implementation Details 🚧

Testing Details 🔍

codecov-io commented Aug 27, 2018 • edited Loading

Codecov Report

jjatie commented Sep 16, 2018

jjatie Sep 16, 2018

Choose a reason for hiding this comment

liuxuan30 left a comment • edited Loading

Choose a reason for hiding this comment

proped commented Oct 10, 2018

AntonTheDev commented Oct 10, 2018

natan commented Oct 23, 2018

pmairoldi commented Oct 23, 2018 • edited Loading

liuxuan30 commented Oct 24, 2018

natan commented Oct 24, 2018

liuxuan30 commented Oct 25, 2018

AntonTheDev commented Oct 26, 2018 • edited Loading

anton-filimonov commented Oct 27, 2018

jjatie commented Oct 27, 2018

liuxuan30 commented Nov 1, 2018 • edited Loading

Arkezis commented Jan 17, 2019

liuxuan30 commented Jan 22, 2019

jlubeck commented Nov 19, 2019

AntonTheDev commented Aug 27, 2018 •

edited

Loading

codecov-io commented Aug 27, 2018 •

edited

Loading

liuxuan30 left a comment •

edited

Loading

pmairoldi commented Oct 23, 2018 •

edited

Loading

AntonTheDev commented Oct 26, 2018 •

edited

Loading

liuxuan30 commented Nov 1, 2018 •

edited

Loading